Add featuregates for volcano capabilities #2989

Monokaix · 2023-07-21T10:55:07Z

Releated issue: #3012
Volcano supports many kubernetes native resources and uses these resources to serve the scheduling algorithm, it also provides high-level capabilities such as queues and queue commands. In serverless scenarios, we only focus on core resources such as pods/podgroup, we should disable some resources and capabilitis by feature gate.
So this pr：

Categorize basic and advanced capabilities of scheduling and add featuregates for advanced capabilities and set to true by default.
User can still use all the volcano capabilities without being aware of the classification, and set the featuregate explicitly to false to disable resources and capabilities that don't need in serveless use case.

Thor-wl · 2023-07-24T02:13:27Z

@Monokaix Thanks for your contribution. I think I've been aware of your requirements and ideas in some degree. Can you tell more about the meaning of this fearure? In another word, what will happen without the PR? Unacceptable memory usage or poor performance due to unusable cache?

Thor-wl · 2023-07-24T02:13:54Z

/hold

Monokaix · 2023-07-24T12:35:17Z

@Monokaix Thanks for your contribution. I think I've been aware of your requirements and ideas in some degree. Can you tell more about the meaning of this fearure? In another word, what will happen without the PR? Unacceptable memory usage or poor performance due to unusable cache?

We hope that Volcano provides a basic framework, which only includes some core resources of k8s. Without this pr, Volcano will not start successfully if apiserver has no advanced capabilities API registered, which are showed in this pr, because it has to wait for the cache synchronization of informer resources.

william-wang · 2023-07-31T02:28:52Z

@Monokaix Thanks for your contribution. I think I've been aware of your requirements and ideas in some degree. Can you tell more about the meaning of this fearure? In another word, what will happen without the PR? Unacceptable memory usage or poor performance due to unusable cache?

We hope that Volcano provides a basic framework, which only includes some core resources of k8s. Without this pr, Volcano will not start successfully if apiserver has no advanced capabilities API registered, which are showed in this pr, because it has to wait for the cache synchronization of informer resources.

Some user builds serverless container platform based on Volcano. The infra platform is a simplied kubernetes that have no object like CSI, priority class and so on. The current Volcano does not play with it. From my side, Volcano can be enhanced to
let user enable or disable the list/watch of CSI, priority class on demand and be more flexable.

@Monokaix suggest to file a enhancement issue and clarify the scenario and associate it with this pr.

Thor-wl · 2023-08-01T07:49:36Z

From my side, Volcano can be enhanced to
let user enable or disable the list/watch of CSI, priority class on demand and be more flexable.

+1

Monokaix · 2023-08-01T11:06:20Z

@Monokaix Thanks for your contribution. I think I've been aware of your requirements and ideas in some degree. Can you tell more about the meaning of this fearure? In another word, what will happen without the PR? Unacceptable memory usage or poor performance due to unusable cache?

We hope that Volcano provides a basic framework, which only includes some core resources of k8s. Without this pr, Volcano will not start successfully if apiserver has no advanced capabilities API registered, which are showed in this pr, because it has to wait for the cache synchronization of informer resources.

Some user builds serverless container platform based on Volcano. The infra platform is a simplied kubernetes that have no object like CSI, priority class and so on. The current Volcano does not play with it. From my side, Volcano can be enhanced to let user enable or disable the list/watch of CSI, priority class on demand and be more flexable.

@Monokaix suggest to file a enhancement issue and clarify the scenario and associate it with this pr.

Thanks, enhancement is added.

Monokaix · 2023-08-02T09:04:18Z

From my side, Volcano can be enhanced to
let user enable or disable the list/watch of CSI, priority class on demand and be more flexable.

+1

@Thor-wl Enhancement has been posted, Could you take a look once more?

Thor-wl · 2023-08-03T11:30:42Z

@Thor-wl Enhancement has been posted, Could you take a look once more?

OK. Since this PR is a little big large, it may cost some time to reivew.

Thor-wl · 2023-08-14T02:16:58Z

cmd/scheduler/app/options/options.go

@@ -60,8 +60,6 @@ type ServerOption struct {
 PrintVersion bool
 EnableMetrics bool
 ListenAddress string
- EnablePriorityClass bool
- EnableCSIStorage bool


Let's take code compatibility into consideration.

Thanks, the params are kept.

Thor-wl · 2023-08-14T02:17:08Z

cmd/scheduler/app/options/options.go

@@ -107,8 +105,6 @@ func (s *ServerOption) AddFlags(fs *pflag.FlagSet) {
 fs.StringVar(&s.LockObjectNamespace, "lock-object-namespace", defaultLockObjectNamespace, "Define the namespace of the lock object that is used for leader election; it is volcano-system by default")
 fs.StringVar(&s.ListenAddress, "listen-address", defaultListenAddress, "The address to listen on for HTTP requests.")
 fs.StringVar(&s.HealthzBindAddress, "healthz-address", defaultHealthzAddress, "The address to listen on for the health check server.")
- fs.BoolVar(&s.EnablePriorityClass, "priority-class", true,


Thanks, the params are kept.

lowang-bh · 2023-08-14T03:12:14Z

Please provide some important test result snapshot about this feature. Thanks.

Thor-wl · 2023-08-15T06:10:18Z

/hold cancel

Thor-wl · 2023-08-15T06:14:29Z

pkg/controllers/job/job_controller.go

+ FilterFunc: func(obj interface{}) bool {
+ switch v := obj.(type) {
+ case *busv1alpha1.Command:
+ if v.TargetObject != nil &&


Suggest to be simpled as return v.TargetObject != nil && v.TargetObject.APIVersion == batchv1alpha1.SchemeGroupVersion.String() && v.TargetObject.Kind == "Job"

Thor-wl · 2023-08-15T06:18:49Z

pkg/features/volcano_features.go

+)
+
+const (
+ // WorkLoadSupport can cache and operate workload resource, Deployment/Replicas/ReplicationController/StatefulSet resources currently.


Suggest to update the comment to be WorkLoadSupport can cache and operate **K8s native resource**, Deployment/Replicas/ReplicationController/StatefulSet resources currently.. That may be easier to understand.

Signed-off-by: Xuzheng Chang <[email protected]>

Thor-wl · 2023-08-16T06:27:05Z

/lgtm

william-wang · 2023-08-16T07:11:14Z

pkg/scheduler/cache/cache.go

@@ -33,6 +33,7 @@ import (
 utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 "k8s.io/apimachinery/pkg/util/sets"
 "k8s.io/apimachinery/pkg/util/wait"
+ utilfeature "k8s.io/apiserver/pkg/util/feature"


please change utilfeature to be a more reasonable name

william-wang · 2023-08-16T07:13:05Z

pkg/features/volcano_features.go

+ runtime.Must(utilfeature.DefaultMutableFeatureGate.Add(defaultVolcanoFeatureGates))
+}
+
+var defaultVolcanoFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{


change defaultVolcanoFeatureGates to defaultFeatureGates

william-wang

/approve

volcano-sh-bot · 2023-08-16T07:30:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: william-wang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [william-wang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

volcano-sh-bot requested review from hwdef, jiangkaihua and shinytang6 July 21, 2023 10:55

volcano-sh-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 21, 2023

Monokaix force-pushed the featuregate branch 2 times, most recently from 289c921 to eb2c40f Compare July 21, 2023 11:21

volcano-sh-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 24, 2023

Monokaix force-pushed the featuregate branch from eb2c40f to 8809f2d Compare July 24, 2023 10:40

Monokaix force-pushed the featuregate branch from 8809f2d to cbf0725 Compare July 25, 2023 08:28

Monokaix force-pushed the featuregate branch 3 times, most recently from 1695d41 to a36dc1b Compare August 1, 2023 06:36

Monokaix force-pushed the featuregate branch 4 times, most recently from 05ee33a to 9d4bdd3 Compare August 1, 2023 11:00

Monokaix force-pushed the featuregate branch 2 times, most recently from 400a886 to 6150894 Compare August 2, 2023 07:29

Monokaix force-pushed the featuregate branch from 6150894 to 2fa23d9 Compare August 11, 2023 08:09

Thor-wl reviewed Aug 14, 2023

View reviewed changes

Monokaix force-pushed the featuregate branch from 2fa23d9 to 06df76a Compare August 14, 2023 06:33

Monokaix force-pushed the featuregate branch from 06df76a to be9ff13 Compare August 14, 2023 06:38

volcano-sh-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 15, 2023

Thor-wl reviewed Aug 15, 2023

View reviewed changes

Add featuregates for volcano capabilities

76c70f2

Signed-off-by: Xuzheng Chang <[email protected]>

Monokaix force-pushed the featuregate branch from be9ff13 to 76c70f2 Compare August 16, 2023 06:09

volcano-sh-bot assigned Thor-wl Aug 16, 2023

volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 16, 2023

william-wang reviewed Aug 16, 2023

View reviewed changes

william-wang approved these changes Aug 16, 2023

View reviewed changes

volcano-sh-bot assigned william-wang Aug 16, 2023

volcano-sh-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 16, 2023

volcano-sh-bot merged commit 433e976 into volcano-sh:master Aug 16, 2023
12 checks passed

Monokaix mentioned this pull request Nov 25, 2023

Request to be an Reviewer of Volcano community volcano-sh/community#44

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add featuregates for volcano capabilities #2989

Add featuregates for volcano capabilities #2989

Monokaix commented Jul 21, 2023 •

edited

Loading

Thor-wl commented Jul 24, 2023

Thor-wl commented Jul 24, 2023

Monokaix commented Jul 24, 2023 •

edited

Loading

william-wang commented Jul 31, 2023

Thor-wl commented Aug 1, 2023

Monokaix commented Aug 1, 2023

Monokaix commented Aug 2, 2023 •

edited

Loading

Thor-wl commented Aug 3, 2023

Thor-wl Aug 14, 2023

Monokaix Aug 14, 2023 •

edited

Loading

Thor-wl Aug 14, 2023

Monokaix Aug 14, 2023

lowang-bh commented Aug 14, 2023 •

edited

Loading

Thor-wl commented Aug 15, 2023

Thor-wl Aug 15, 2023

Thor-wl Aug 15, 2023 •

edited

Loading

Thor-wl commented Aug 16, 2023

william-wang Aug 16, 2023 •

edited

Loading

william-wang Aug 16, 2023

william-wang left a comment

volcano-sh-bot commented Aug 16, 2023

Add featuregates for volcano capabilities #2989

Add featuregates for volcano capabilities #2989

Conversation

Monokaix commented Jul 21, 2023 • edited Loading

Thor-wl commented Jul 24, 2023

Thor-wl commented Jul 24, 2023

Monokaix commented Jul 24, 2023 • edited Loading

william-wang commented Jul 31, 2023

Thor-wl commented Aug 1, 2023

Monokaix commented Aug 1, 2023

Monokaix commented Aug 2, 2023 • edited Loading

Thor-wl commented Aug 3, 2023

Thor-wl Aug 14, 2023

Choose a reason for hiding this comment

Monokaix Aug 14, 2023 • edited Loading

Choose a reason for hiding this comment

Thor-wl Aug 14, 2023

Choose a reason for hiding this comment

Monokaix Aug 14, 2023

Choose a reason for hiding this comment

lowang-bh commented Aug 14, 2023 • edited Loading

Thor-wl commented Aug 15, 2023

Thor-wl Aug 15, 2023

Choose a reason for hiding this comment

Thor-wl Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

Thor-wl commented Aug 16, 2023

william-wang Aug 16, 2023 • edited Loading

Choose a reason for hiding this comment

william-wang Aug 16, 2023

Choose a reason for hiding this comment

william-wang left a comment

Choose a reason for hiding this comment

volcano-sh-bot commented Aug 16, 2023

Monokaix commented Jul 21, 2023 •

edited

Loading

Monokaix commented Jul 24, 2023 •

edited

Loading

Monokaix commented Aug 2, 2023 •

edited

Loading

Monokaix Aug 14, 2023 •

edited

Loading

lowang-bh commented Aug 14, 2023 •

edited

Loading

Thor-wl Aug 15, 2023 •

edited

Loading

william-wang Aug 16, 2023 •

edited

Loading